feat: implement right sidebar inspector panel#9
Conversation
datlechin
commented
Dec 25, 2025
- Add RightSidebarView with table metadata and row details
- Add TableMetadata model with size, stats, and timestamps
- Add fetchTableMetadata to database drivers (MySQL, PostgreSQL, SQLite)
- Toggle right sidebar with Cmd+Opt+B
- Conditional HStack rendering for proper collapse behavior
- Professional macOS inspector-style UI with sections
- Add RightSidebarView with table metadata and row details - Add TableMetadata model with size, stats, and timestamps - Add fetchTableMetadata to database drivers (MySQL, PostgreSQL, SQLite) - Toggle right sidebar with Cmd+Opt+B - Conditional HStack rendering for proper collapse behavior - Professional macOS inspector-style UI with sections
There was a problem hiding this comment.
Pull request overview
This PR implements a right sidebar inspector panel for displaying table metadata and selected row details, accessible via Cmd+Opt+B keyboard shortcut.
Key Changes:
- Added RightSidebarView with professional macOS inspector-style UI showing table statistics and selected row field details
- Implemented TableMetadata model and fetchTableMetadata() method across all database drivers (MySQL, PostgreSQL, SQLite)
- Integrated right sidebar toggle via NotificationCenter with conditional HStack rendering for smooth collapse behavior
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| OpenTable/Views/RightSidebar/RightSidebarView.swift | New inspector panel UI with table metadata sections, row details, and search filtering |
| OpenTable/Models/TableMetadata.swift | Data model for table size, statistics, and timestamps with size formatting utilities |
| OpenTable/Core/Database/MySQLDriver.swift | Fetches metadata via SHOW TABLE STATUS including engine, collation, and dates |
| OpenTable/Core/Database/PostgreSQLDriver.swift | Queries pg_class/pg_namespace for table sizes and row statistics |
| OpenTable/Core/Database/SQLiteDriver.swift | Estimates metadata from file size and row count queries |
| OpenTable/Core/Database/DatabaseDriver.swift | Added fetchTableMetadata protocol requirement |
| OpenTable/Views/MainContentView.swift | Integrated right sidebar with HStack layout, metadata loading, and row data computation |
| OpenTable/ContentView.swift | Added inspector state binding and changed default visibility to doubleColumn |
| OpenTable/Views/Toolbar/OpenTableToolbarView.swift | Replaced "not implemented" alert with NotificationCenter toggle action |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Escape single quotes in table names for PostgreSQL and SQLite - Use safeTableName.replacingOccurrences(of: "'", with: "''") to prevent injection
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Fix SQLite SQL injection: use double quotes for identifiers - Fix PostgreSQL SQL injection: proper single-quote escaping - Add static DateFormatter in MySQLDriver for performance - Use .min() instead of .sorted().first in MainContentView - ForEach use column name as stable id instead of offset - Only show search field for row details (not table metadata) - Remove unused displayProperties from TableMetadata
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
SQLite doesn't expose accurate per-table size information. The database file size was incorrectly shown as table size. Now setting size-related fields to nil to avoid confusion.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Check if metadata already loaded before fetching (tableName comparison) - Both .task(id:) and toggle handler now skip if already loaded - Fixed comment for 2-column NavigationSplitView
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| "fill" : { | ||
| "automatic-gradient" : "srgb:1.00000,0.57637,0.00000,1.00000" | ||
| "fill": { | ||
| "automatic-gradient": "srgb:1.00000,0.57637,0.00000,1.00000" | ||
| }, | ||
| "groups" : [ | ||
| "groups": [ | ||
| { | ||
| "layers" : [ | ||
| "layers": [ | ||
| { | ||
| "fill" : { | ||
| "solid" : "display-p3:0.99736,1.00000,0.97886,1.00000" | ||
| "fill": { | ||
| "solid": "display-p3:0.99736,1.00000,0.97886,1.00000" | ||
| }, | ||
| "image-name" : "cylinder.split.1x2.fill 1.svg", | ||
| "name" : "cylinder.split.1x2.fill 1" | ||
| "image-name": "cylinder.split.1x2.fill 1.svg", | ||
| "name": "cylinder.split.1x2.fill 1" | ||
| } | ||
| ], | ||
| "position" : { | ||
| "scale" : 1.3, | ||
| "translation-in-points" : [ | ||
| "position": { | ||
| "scale": 1.3, | ||
| "translation-in-points": [ | ||
| 0, | ||
| 0 | ||
| ] | ||
| }, | ||
| "shadow" : { | ||
| "kind" : "neutral", | ||
| "opacity" : 0.5 | ||
| "shadow": { | ||
| "kind": "neutral", | ||
| "opacity": 0.5 | ||
| }, | ||
| "specular" : false, | ||
| "translucency" : { | ||
| "enabled" : true, | ||
| "value" : 0.5 | ||
| "specular": false, | ||
| "translucency": { | ||
| "enabled": true, | ||
| "value": 0.5 | ||
| } | ||
| } | ||
| ], | ||
| "supported-platforms" : { | ||
| "circles" : [ | ||
| "supported-platforms": { | ||
| "circles": [ | ||
| "watchOS" | ||
| ], | ||
| "squares" : "shared" | ||
| "squares": "shared" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This file appears to have been reformatted with spacing changes (spaces after colons added throughout) but no functional changes. This seems unintentional as it's not mentioned in the PR description and is unrelated to the inspector panel feature. Consider reverting these formatting changes to avoid unnecessary noise in the diff.
| // Load table metadata only if opening and not already loaded for this table | ||
| if isInspectorPresented, | ||
| let tableName = currentTab?.tableName, | ||
| tableMetadata?.tableName != tableName { | ||
| Task { await loadTableMetadata(tableName: tableName) } | ||
| } |
There was a problem hiding this comment.
There's duplicate logic for loading table metadata: once in the .task modifier when the tab changes (lines 104-113) and once in the onReceive handler when the inspector is toggled (lines 378-383). Both check the same conditions (whether the table name matches the current metadata). This could lead to unnecessary duplicate API calls if both conditions trigger simultaneously. Consider consolidating this logic into a single function or removing one of these redundant checks.
| // Load table metadata only if opening and not already loaded for this table | |
| if isInspectorPresented, | |
| let tableName = currentTab?.tableName, | |
| tableMetadata?.tableName != tableName { | |
| Task { await loadTableMetadata(tableName: tableName) } | |
| } |
| } catch { | ||
| print("[MainContentView] Failed to load table metadata: \(error)") | ||
| } |
There was a problem hiding this comment.
Error handling only logs to console with print, which makes it difficult for users to know why table metadata failed to load. The inspector panel will silently show no metadata on error. Consider displaying an error state in the UI (e.g., an error message in the inspector panel) or using proper logging infrastructure to make debugging easier.
| private var searchField: some View { | ||
| HStack(spacing: 6) { | ||
| Image(systemName: "magnifyingglass") | ||
| .foregroundStyle(.tertiary) | ||
| .font(.system(size: 10)) | ||
|
|
||
| TextField("Filter", text: $searchText) | ||
| .textFieldStyle(.plain) | ||
| .font(.system(size: 11)) | ||
|
|
||
| if !searchText.isEmpty { | ||
| Button(action: { searchText = "" }) { | ||
| Image(systemName: "xmark.circle.fill") | ||
| .foregroundStyle(.tertiary) | ||
| .font(.system(size: 10)) | ||
| } | ||
| .buttonStyle(.plain) | ||
| } | ||
| } | ||
| .padding(.horizontal, 12) | ||
| .padding(.vertical, 6) | ||
| .background(Color(NSColor.textBackgroundColor).opacity(0.5)) | ||
| } |
There was a problem hiding this comment.
The search field and clear button lack accessibility labels, which could make the inspector panel harder to use with VoiceOver or other assistive technologies. Consider adding .accessibilityLabel() modifiers to the search field and clear button to describe their purpose.
| // Simple type indicator - can be enhanced later with actual column type info | ||
| let type = "string" |
There was a problem hiding this comment.
The type is hard-coded as "string" for all columns regardless of the actual column data type. This provides limited value to users inspecting row details. Consider retrieving actual column type information from the result metadata or the schema. The comment acknowledges this can be "enhanced later", but displaying incorrect type information could mislead users.
| // Simple type indicator - can be enhanced later with actual column type info | |
| let type = "string" | |
| // Type is currently unknown here; can be enhanced later with actual column type info | |
| let type = "unknown" |
| FROM pg_class c | ||
| JOIN pg_namespace n ON n.oid = c.relnamespace | ||
| WHERE c.relname = '\(safeTableName)' | ||
| AND n.nspname = 'public' |
There was a problem hiding this comment.
The query hardcodes the schema name as 'public', which may not work correctly for tables in other schemas. Users working with tables in custom schemas (e.g., 'myapp', 'staging') will not be able to view metadata for those tables. Consider either accepting the schema name as a parameter, using the search_path to determine the appropriate schema, or querying across all accessible schemas.
| AND n.nspname = 'public' | |
| AND pg_table_is_visible(c.oid) |
| // NOTE: `SHOW TABLE STATUS LIKE` expects a pattern string literal, not an | ||
| // identifier. For that reason we must use single-quoted string syntax here | ||
| // instead of the backtick identifier quoting used in other schema queries | ||
| // (e.g. `SHOW CREATE TABLE \`table\``). The table name is safely embedded | ||
| // by escaping single quotes above. |
There was a problem hiding this comment.
The comment states "SHOW TABLE STATUS LIKE expects a pattern string literal" but the query actually uses "WHERE Name =" which is an equality comparison, not a LIKE pattern match. The comment should be updated to reflect that this is a WHERE clause with string comparison, not a LIKE pattern. The escaping is correct for preventing SQL injection in string literals.
| // NOTE: `SHOW TABLE STATUS LIKE` expects a pattern string literal, not an | |
| // identifier. For that reason we must use single-quoted string syntax here | |
| // instead of the backtick identifier quoting used in other schema queries | |
| // (e.g. `SHOW CREATE TABLE \`table\``). The table name is safely embedded | |
| // by escaping single quotes above. | |
| // NOTE: This query filters `SHOW TABLE STATUS` results using a WHERE clause | |
| // on the Name column. The table name is embedded as a single-quoted string | |
| // literal (not as a backtick-quoted identifier), which is why we escape | |
| // single quotes above before constructing the query string. |
| func fetchTableMetadata(tableName: String) async throws -> TableMetadata { | ||
| let escapedTableName = tableName.replacingOccurrences(of: "'", with: "''") | ||
| // NOTE: `SHOW TABLE STATUS LIKE` expects a pattern string literal, not an | ||
| // identifier. For that reason we must use single-quoted string syntax here | ||
| // instead of the backtick identifier quoting used in other schema queries | ||
| // (e.g. `SHOW CREATE TABLE \`table\``). The table name is safely embedded | ||
| // by escaping single quotes above. |
There was a problem hiding this comment.
The fetchTableMetadata implementation builds a MySQL query by interpolating tableName into a single-quoted string and only escapes single quotes, which is not sufficient to prevent SQL injection in MySQL/MariaDB string literals. Because backslash escapes are enabled by default, a crafted table name or value such as abc\'; DROP TABLE important;-- can use \' to terminate the string despite the single-quote doubling and inject additional statements, executed with the full privileges of this connection. To fix this, avoid manual string concatenation here by using parameterized queries or the driver’s native literal-escaping API, or at minimum also correctly escape backslashes and other special characters when embedding identifiers as string literals.
| func fetchTableMetadata(tableName: String) async throws -> TableMetadata { | |
| let escapedTableName = tableName.replacingOccurrences(of: "'", with: "''") | |
| // NOTE: `SHOW TABLE STATUS LIKE` expects a pattern string literal, not an | |
| // identifier. For that reason we must use single-quoted string syntax here | |
| // instead of the backtick identifier quoting used in other schema queries | |
| // (e.g. `SHOW CREATE TABLE \`table\``). The table name is safely embedded | |
| // by escaping single quotes above. | |
| /// Escape a value for safe use as a single-quoted MySQL/MariaDB string literal. | |
| /// This handles backslashes and single quotes according to MySQL string rules. | |
| private func escapeMySQLStringLiteral(_ value: String) -> String { | |
| // Order is important: escape backslashes first, then single quotes. | |
| var escaped = value.replacingOccurrences(of: "\\", with: "\\\\") | |
| escaped = escaped.replacingOccurrences(of: "'", with: "''") | |
| return escaped | |
| } | |
| func fetchTableMetadata(tableName: String) async throws -> TableMetadata { | |
| let escapedTableName = escapeMySQLStringLiteral(tableName) | |
| // NOTE: `SHOW TABLE STATUS WHERE Name =` expects a pattern string literal, | |
| // not an identifier. For that reason we must use single-quoted string syntax | |
| // here instead of the backtick identifier quoting used in other schema | |
| // queries (e.g. `SHOW CREATE TABLE \`table\``). The table name is safely | |
| // embedded by escaping backslashes and single quotes above. |